Skip to content

Conversation

@m1kola
Copy link
Member

@m1kola m1kola commented Jun 28, 2024

Description

Closes #989 by removing the condition and solves an E2E flake.

Note: in this brief were are revisiting ClusterExtension conditions and their reasons. One of the points is about removal of HasValidBundle condition type.

The brief is still in review, but I would like to move forward with this change as it resolves a flake I hit regularly (especially locally).

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

@openshift-ci openshift-ci bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 28, 2024
@netlify
Copy link

netlify bot commented Jun 28, 2024

Deploy Preview for olmv1 ready!

Name Link
🔨 Latest commit d1bda18
🔍 Latest deploy log https://app.netlify.com/sites/olmv1/deploys/667eb2afa90d6a00086bfb8e
😎 Deploy Preview https://deploy-preview-990--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@codecov
Copy link

codecov bot commented Jun 28, 2024

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 77.19%. Comparing base (cab41aa) to head (d1bda18).

Files Patch % Lines
...nternal/controllers/clusterextension_controller.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #990      +/-   ##
==========================================
+ Coverage   76.89%   77.19%   +0.29%     
==========================================
  Files          17       17              
  Lines        1225     1206      -19     
==========================================
- Hits          942      931      -11     
+ Misses        201      193       -8     
  Partials       82       82              
Flag Coverage Δ
e2e 56.96% <0.00%> (+0.72%) ⬆️
unit 51.90% <0.00%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@m1kola m1kola marked this pull request as ready for review June 28, 2024 12:33
@m1kola m1kola requested a review from a team as a code owner June 28, 2024 12:34
@openshift-ci openshift-ci bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 28, 2024
@m1kola
Copy link
Member Author

m1kola commented Jun 28, 2024

go-apidiff is expected to fail because I'm chasing the API here.

@m1kola m1kola requested a review from bentito June 28, 2024 12:42
Signed-off-by: Mikalai Radchuk <[email protected]>
@m1kola m1kola force-pushed the remove_HasValidBundle_condition branch from 32fde13 to d1bda18 Compare June 28, 2024 12:55
@bentito
Copy link
Contributor

bentito commented Jun 28, 2024

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jun 28, 2024
@m1kola m1kola mentioned this pull request Jun 28, 2024
4 tasks
Copy link
Contributor

@ankitathomas ankitathomas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@m1kola m1kola added this pull request to the merge queue Jun 28, 2024
Merged via the queue into operator-framework:main with commit 97fb589 Jun 28, 2024
@m1kola m1kola deleted the remove_HasValidBundle_condition branch June 28, 2024 14:16
perdasilva pushed a commit to LalatenduMohanty/operator-controller that referenced this pull request Aug 13, 2024
@skattoju skattoju mentioned this pull request Sep 25, 2024
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

lgtm Indicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ClusterExtension's condition HasValidBundle is False when Resolved, Installed and Unpacked are all True

3 participants